pull: Fold together deltapart+fallback count for display

It's just simpler, and I'm not sure people are going to care
much about the difference by default.

We already folded in the fallback sizes into the download totals, so folding in
the count makes things consistent; previously you could see e.g.
`3/3 parts, 100MB/150MB` and be confused.

Closes: #678
Approved by: giuseppe

diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c
index 6246011fd561bf73de2f2a3fcce8d8ba2f0ce1c2..3452fbe9c25086352af1f42c69103d786d34088d 100644 (file)
--- a/src/libostree/ostree-repo-pull.c
+++ b/src/libostree/ostree-repo-pull.c
@@ -81,6 +81,7 @@ typedef struct {
   GHashTable       *scanned_metadata; /* Maps object name to itself */
   GHashTable       *requested_metadata; /* Maps object name to itself */
   GHashTable       *requested_content; /* Maps checksum to itself */
+  GHashTable       *requested_fallback_content; /* Maps checksum to itself */
   GHashTable       *pending_fetch_metadata; /* Map<ObjectName,FetchObjectData> */
   GHashTable       *pending_fetch_content; /* Map<checksum,FetchObjectData> */
   GHashTable       *pending_fetch_deltaparts; /* Set<FetchStaticDeltaData> */
@@ -98,6 +99,7 @@ typedef struct {
   gint              n_requested_metadata;
   gint              n_requested_content;
   guint             n_fetched_deltaparts;
+  guint             n_fetched_deltapart_fallbacks;
   guint             n_fetched_metadata;
   guint             n_fetched_content;
 
@@ -222,6 +224,8 @@ update_progress (gpointer user_data)
                                   pull_data->n_fetched_deltaparts);
   ostree_async_progress_set_uint (pull_data->progress, "total-delta-parts",
                                   pull_data->n_total_deltaparts);
+  ostree_async_progress_set_uint (pull_data->progress, "fetched-delta-fallbacks",
+                                  pull_data->n_fetched_deltapart_fallbacks);
   ostree_async_progress_set_uint (pull_data->progress, "total-delta-fallbacks",
                                   pull_data->n_total_delta_fallbacks);
   ostree_async_progress_set_uint64 (pull_data->progress, "fetched-delta-part-size",
@@ -785,6 +789,9 @@ content_fetch_on_write_complete (GObject        *object,
     }
 
   pull_data->n_fetched_content++;
+  /* Was this a delta fallback? */
+  if (g_hash_table_remove (pull_data->requested_fallback_content, expected_checksum))
+    pull_data->n_fetched_deltapart_fallbacks++;
  out:
   pull_data->n_outstanding_content_write_requests--;
   check_outstanding_requests_handle_error (pull_data, local_error);
@@ -1631,9 +1638,14 @@ process_one_static_delta_fallback (OtPullData   *pull_data,
         {
           if (!g_hash_table_lookup (pull_data->requested_content, checksum))
             {
+              /* Mark this as requested, like we do in the non-delta path */
               g_hash_table_add (pull_data->requested_content, checksum);
+              /* But also record it's a delta fallback object, so we can account
+               * for it as logically part of the delta fetch.
+               */
+              g_hash_table_add (pull_data->requested_fallback_content, g_strdup (checksum));
               enqueue_one_object_request (pull_data, checksum, OSTREE_OBJECT_TYPE_FILE, NULL, FALSE, FALSE);
-              checksum = NULL;  /* Transfer ownership */
+              checksum = NULL;  /* We transferred ownership to the requested_content hash */
             }
         }
     }
@@ -2604,6 +2616,8 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
                                                        (GDestroyNotify)g_variant_unref, NULL);
   pull_data->requested_content = g_hash_table_new_full (g_str_hash, g_str_equal,
                                                         (GDestroyNotify)g_free, NULL);
+  pull_data->requested_fallback_content = g_hash_table_new_full (g_str_hash, g_str_equal,
+                                                                 (GDestroyNotify)g_free, NULL);
   pull_data->requested_metadata = g_hash_table_new_full (ostree_hash_object_name, g_variant_equal,
                                                          (GDestroyNotify)g_variant_unref, NULL);
   pull_data->pending_fetch_content = g_hash_table_new_full (g_str_hash, g_str_equal,
@@ -3324,6 +3338,7 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
   g_clear_pointer (&pull_data->scanned_metadata, (GDestroyNotify) g_hash_table_unref);
   g_clear_pointer (&pull_data->summary_deltas_checksums, (GDestroyNotify) g_hash_table_unref);
   g_clear_pointer (&pull_data->requested_content, (GDestroyNotify) g_hash_table_unref);
+  g_clear_pointer (&pull_data->requested_fallback_content, (GDestroyNotify) g_hash_table_unref);
   g_clear_pointer (&pull_data->requested_metadata, (GDestroyNotify) g_hash_table_unref);
   g_clear_pointer (&pull_data->pending_fetch_content, (GDestroyNotify) g_hash_table_unref);
   g_clear_pointer (&pull_data->pending_fetch_metadata, (GDestroyNotify) g_hash_table_unref);

diff --git a/src/libostree/ostree-repo-static-delta-core.c b/src/libostree/ostree-repo-static-delta-core.c
index 1ab11ed771d57cd24476da5f86f2cf633dab6951..21ed081dbfbfdb5dc7139c045dbbdf969a0066ed 100644 (file)
--- a/src/libostree/ostree-repo-static-delta-core.c
+++ b/src/libostree/ostree-repo-static-delta-core.c
@@ -937,7 +937,7 @@ _ostree_repo_static_delta_dump (OstreeRepo                    *self,
       {
         guint64 size, usize;
         g_autoptr(GVariant) checksum_v = NULL;
-        char checksum[65];
+        char checksum[OSTREE_SHA256_STRING_LEN+1];
         g_variant_get_child (fallback, i, "(y@aytt)", NULL, &checksum_v, &size, &usize);
         ostree_checksum_inplace_from_bytes (ostree_checksum_bytes_peek (checksum_v), checksum);
         size = maybe_swap_endian_u64 (swap_endian, size);

diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c
index 98abd9265ad066fa7b99ce57dde4742f4bf24bee..dc0eb57512073af3521a19ee0e1e379f95550706 100644 (file)
--- a/src/libostree/ostree-repo.c
+++ b/src/libostree/ostree-repo.c
@@ -3884,6 +3884,8 @@ ostree_repo_pull_default_console_progress_changed (OstreeAsyncProgress *progress
   guint n_scanned_metadata;
   guint fetched_delta_parts;
   guint total_delta_parts;
+  guint fetched_delta_part_fallbacks;
+  guint total_delta_part_fallbacks;
 
   buf = g_string_new ("");
 
@@ -3895,6 +3897,8 @@ ostree_repo_pull_default_console_progress_changed (OstreeAsyncProgress *progress
   n_scanned_metadata = ostree_async_progress_get_uint (progress, "scanned-metadata");
   fetched_delta_parts = ostree_async_progress_get_uint (progress, "fetched-delta-parts");
   total_delta_parts = ostree_async_progress_get_uint (progress, "total-delta-parts");
+  fetched_delta_part_fallbacks = ostree_async_progress_get_uint (progress, "fetched-delta-fallbacks");
+  total_delta_part_fallbacks = ostree_async_progress_get_uint (progress, "total-delta-fallbacks");
 
   if (status)
     {
@@ -3932,10 +3936,15 @@ ostree_repo_pull_default_console_progress_changed (OstreeAsyncProgress *progress
       if (total_delta_parts > 0)
         {
           guint64 fetched_delta_part_size = ostree_async_progress_get_uint64 (progress, "fetched-delta-part-size");
-          g_autofree char *formatted_fetched =
-            g_format_size (fetched_delta_part_size);
-          g_autofree char *formatted_total =
-            g_format_size (total_delta_part_size);
+          g_autofree char *formatted_fetched = NULL;
+          g_autofree char *formatted_total = NULL;
+
+          /* Here we merge together deltaparts + fallbacks to avoid bloating the text UI */
+          fetched_delta_parts += fetched_delta_part_fallbacks;
+          total_delta_parts += total_delta_part_fallbacks;
+
+          formatted_fetched = g_format_size (fetched_delta_part_size);
+          formatted_total = g_format_size (total_delta_part_size);
 
           if (bytes_sec > 0)
             {

diff --git a/src/ostree/ot-builtin-pull.c b/src/ostree/ot-builtin-pull.c
index 7df48002445cd3a9e73feefe52a9b91b5532d60a..170a3a91d713ca050350f4e17522402f39634174 100644 (file)
--- a/src/ostree/ot-builtin-pull.c
+++ b/src/ostree/ot-builtin-pull.c
@@ -80,14 +80,24 @@ dry_run_console_progress_changed (OstreeAsyncProgress *progress,
                                   gpointer             user_data)
 {
   guint fetched_delta_parts, total_delta_parts;
+  guint fetched_delta_part_fallbacks, total_delta_part_fallbacks;
   guint64 fetched_delta_part_size, total_delta_part_size, total_delta_part_usize;
   GString *buf;
 
   g_assert (!printed_console_progress);
   printed_console_progress = TRUE;
 
+  /* Number of parts */
   fetched_delta_parts = ostree_async_progress_get_uint (progress, "fetched-delta-parts");
   total_delta_parts = ostree_async_progress_get_uint (progress, "total-delta-parts");
+  fetched_delta_part_fallbacks = ostree_async_progress_get_uint (progress, "fetched-delta-fallbacks");
+  total_delta_part_fallbacks = ostree_async_progress_get_uint (progress, "total-delta-fallbacks");
+  /* Fold the count of deltaparts + fallbacks for simplicity; if changing this,
+   * please change ostree_repo_pull_default_console_progress_changed() first.
+   */
+  fetched_delta_parts += fetched_delta_part_fallbacks;
+  total_delta_parts += total_delta_part_fallbacks;
+  /* Size variables */
   fetched_delta_part_size = ostree_async_progress_get_uint64 (progress, "fetched-delta-part-size");
   total_delta_part_size = ostree_async_progress_get_uint64 (progress, "total-delta-part-size");
   total_delta_part_usize = ostree_async_progress_get_uint64 (progress, "total-delta-part-usize");

diff --git a/tests/libtest.sh b/tests/libtest.sh
index 1c603d66aaf7c43126af733564458b0dba26f08d..ce41668f4a69627fc5b574473701230242051866 100755 (executable)
--- a/tests/libtest.sh
+++ b/tests/libtest.sh
@@ -313,6 +313,10 @@ setup_exampleos_repo() {
         nfiles=10
         while [ $nfiles -gt 0 ]; do
             echo file-$ndirs-$nfiles > f$ndirs-$nfiles
+            # Make an unreadable file to trigger https://github.com/ostreedev/ostree/pull/634
+            if [ $(($x % 10)) -eq 0 ]; then
+                chmod 0600 f$ndirs-$nfiles
+            fi
             nfiles=$((nfiles-1))
         done
         ndirs=$((ndirs-1))

diff --git a/tests/pull-test.sh b/tests/pull-test.sh
index 56b24a0c4d589262a5e973ac1e6aa0532e393383..693eac06ebf5c08cfdbf2542f63a8ae84184abdf 100644 (file)
--- a/tests/pull-test.sh
+++ b/tests/pull-test.sh
@@ -144,7 +144,8 @@ cd ${test_tmpdir}
 repo_init
 ${CMD_PREFIX} ostree --repo=repo pull origin main@${prev_rev}
 ${CMD_PREFIX} ostree --repo=repo pull --dry-run --require-static-deltas origin main >dry-run-pull.txt
-assert_file_has_content dry-run-pull.txt 'Delta update: 0/1 parts'
+# Compression can vary, so we support 400-699
+assert_file_has_content dry-run-pull.txt 'Delta update: 0/1 parts, 0 bytes/[456][0-9][0-9] bytes, 455 bytes total uncompressed'
 rev=$(${CMD_PREFIX} ostree --repo=repo rev-parse origin:main)
 assert_streq "${prev_rev}" "${rev}"
 ${CMD_PREFIX} ostree --repo=repo fsck

diff --git a/tests/test-pull-many.sh b/tests/test-pull-many.sh
index f39c8e6e13533d5bf3ad6e133f4d26456bcb3916..73245a6b23e7d380d604fd08a4f283bf2cab6f6b 100755 (executable)
--- a/tests/test-pull-many.sh
+++ b/tests/test-pull-many.sh
@@ -23,15 +23,30 @@ set -euo pipefail
 
 setup_exampleos_repo
 
-echo '1..1'
+echo '1..3'
 
 cd ${test_tmpdir}
 set -x
 
 echo "$(date): Pulling content..."
 rev=$(${CMD_PREFIX} ostree --repo=ostree-srv/exampleos/repo rev-parse ${REF})
-${CMD_PREFIX} ostree --repo=repo pull --disable-static-deltas --mirror origin ${REF}
+${CMD_PREFIX} ostree --repo=repo pull --disable-static-deltas origin ${REF}
 ${CMD_PREFIX} ostree --repo=repo fsck
 assert_streq ${rev} $(${CMD_PREFIX} ostree --repo=repo rev-parse ${REF})
 
+echo "ok without deltas"
+
+previous=$(${CMD_PREFIX} ostree --repo=repo rev-parse ${rev}^)
+rm repo/refs/{heads,remotes}/* -rf
+${CMD_PREFIX} ostree --repo=repo prune --refs-only
+${CMD_PREFIX} ostree --repo=repo pull origin ${REF}@${previous}
+${CMD_PREFIX} ostree --repo=repo pull --dry-run --require-static-deltas origin ${REF} > output.txt
+assert_file_has_content output.txt 'Delta update: 0/1 parts, 0 bytes/1.[012] MB, 1.[345] MB total uncompressed'
+
+echo "ok delta dry-run"
+
+${CMD_PREFIX} ostree --repo=repo pull  --require-static-deltas origin ${REF}
+assert_streq $(${CMD_PREFIX} ostree --repo=repo rev-parse ${REF}) ${rev}
+${CMD_PREFIX} ostree --repo=repo fsck
+
 echo "ok"